Skip to content

Add concat_then_split packing in Grain pipeline #2786

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
aireenmei wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add concat_then_split packing in Grain pipeline #2786

aireenmei wants to merge 1 commit into from

Conversation

@aireenmei
Copy link
Collaborator

@aireenmei aireenmei commented Dec 4, 2025 •
edited
Loading

Description

b/452438529
See grain docs for the two packing types: first_fit, concat_then_split

Tests

Manually tested both packing methods on v5p

Checklist

Before submitting this PR, please make sure (put X in square brackets):

  • I have performed a self-review of my code. For an optional AI review, add the gemini-review label.
  • I have necessary comments in my code, particularly in hard-to-understand areas.
  • I have run end-to-end tests tests and provided workload links above if applicable.
  • I have made or will make corresponding changes to the doc if needed, including adding new documentation pages to the relevant Table of Contents (toctree directive) as explained in our documentation.

@aireenmei aireenmei force-pushed the aireen/concat_split branch from 1294738 to 7dd5978 Compare December 4, 2025 04:16
@aireenmei aireenmei marked this pull request as ready for review December 4, 2025 04:16
@aireenmei aireenmei force-pushed the aireen/concat_split branch 2 times, most recently from 17ef77a to 8c412e5 Compare December 4, 2025 22:18
NicoGrande
NicoGrande approved these changes Dec 5, 2025
View reviewed changes
Copy link
Collaborator

@NicoGrande NicoGrande left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Only a few minor comments.

hengtaoguo
hengtaoguo approved these changes Dec 5, 2025
View reviewed changes
@aireenmei aireenmei force-pushed the aireen/concat_split branch 2 times, most recently from 42648ba to b983c6b Compare December 9, 2025 00:42
@aireenmei aireenmei force-pushed the aireen/concat_split branch from b983c6b to 53d7829 Compare December 9, 2025 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hengtaoguo hengtaoguo hengtaoguo approved these changes

@NicoGrande NicoGrande NicoGrande approved these changes

@SurbhiJainUSC SurbhiJainUSC Awaiting requested review from SurbhiJainUSC SurbhiJainUSC is a code owner

@richjames0 richjames0 Awaiting requested review from richjames0 richjames0 is a code owner

@shralex shralex Awaiting requested review from shralex shralex is a code owner

@gobbleturk gobbleturk Awaiting requested review from gobbleturk gobbleturk is a code owner

@khatwanimohit khatwanimohit Awaiting requested review from khatwanimohit khatwanimohit is a code owner

@bvandermoon bvandermoon Awaiting requested review from bvandermoon bvandermoon is a code owner

@vipannalla vipannalla Awaiting requested review from vipannalla vipannalla is a code owner

@RissyRan RissyRan Awaiting requested review from RissyRan RissyRan is a code owner

@gagika gagika Awaiting requested review from gagika gagika is a code owner

@A9isha A9isha Awaiting requested review from A9isha A9isha is a code owner

@NuojCheng NuojCheng Awaiting requested review from NuojCheng NuojCheng is a code owner

@jiangjy1982 jiangjy1982 Awaiting requested review from jiangjy1982 jiangjy1982 is a code owner

@suexu1025 suexu1025 Awaiting requested review from suexu1025 suexu1025 is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@aireenmei @hengtaoguo @NicoGrande